Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: xen: gnttab: process gentabs in reverse order #109

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

GrygiriiS
Copy link

The Xen extends domain gentab region every time domain requests gentab basing on gentab idx. If idx > xen_current_max_gentab_idx the Xen extends number of gentabs so that idx <= xen_current_max_gentab_idx. As result now when gentabs are requested from gentab low_idx to max_idx Xen is performing gentab extension by 1 and the bunch of log messages is produced:

(XEN) xen-source/xen/common/grant_table.c:1909:d0v0 Expanding d0
grant table from 1 to 2 frames

This patch changes gentab processing from gentab max_idx to low_idx which allows Xen to perform gentab extension only once (to max_idx) and only one message will be produced in the log:

(XEN) xen-source/xen/common/grant_table.c:1909:d0v0 Expanding d0
grant table from 1 to 64 frames

@GrygiriiS
Copy link
Author

Copy link
Collaborator

@lorc lorc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(XEN) xen-source/xen/common/grant_table.c:1909:d0v0 Expanding d0 \
  grant table from 1 to 2 frames

This message is being printed only if Xen is being build in debug mode. So it is not a problem in production mode.

I don't believe that this patch will get into Zephyr mainline. You can try, though.

But as you are trying to include it into rpi5_dev branch, I have no objections.

Acked-by: Volodymyr Babchuk <[email protected]>

@GrygiriiS
Copy link
Author

GrygiriiS commented Jun 10, 2024

The Linux Kernel has the following:

static int gnttab_map(unsigned int start_idx, unsigned int end_idx)
{
...
	unsigned int nr_gframes = end_idx + 1;
	int rc;

	if (xen_feature(XENFEAT_auto_translated_physmap)) {
		struct xen_add_to_physmap xatp;
		unsigned int i = end_idx;
		rc = 0;
		BUG_ON(xen_auto_xlat_grant_frames.count < nr_gframes);
		/*
		 * Loop backwards, so that the first hypercall has the largest
		 * index, ensuring that the table will grow only once.
		 */
		do {
			xatp.domid = DOMID_SELF;
			xatp.idx = i;
			xatp.space = XENMAPSPACE_grant_table;
			xatp.gpfn = xen_auto_xlat_grant_frames.pfn[i];
			rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);

So, nothing special here really - just a bit faster and less logs.

@lorc
Copy link
Collaborator

lorc commented Jun 10, 2024

		/*
		 * Loop backwards, so that the first hypercall has the largest
		 * index, ensuring that the table will grow only once.
		 */

This is a better justification than "avoid printing debug logs". I believe, if you'll rewrite the commit message WRT to this, it would be much better.

Copy link
Collaborator

@firscity firscity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also in addition to @lorc comment, please, use gnttab (grant table) instead of gentab

@GrygiriiS GrygiriiS force-pushed the rpi5_dev_gnt_order branch from 7a0e74b to c5beeb7 Compare June 13, 2024 09:34
Copy link
Collaborator

@firscity firscity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Dmytro Firsov <[email protected]>

The Xen extends domain grant tables every time domain requests gnttab
basing on gnttab idx. If idx > xen_current_max_gnttab_idx the Xen extends
grant table so that idx <= xen_current_max_gnttab_idx. The growing grant
tables on every hypercall is a bit costly operation and it also results in
the bunch of log messages:

(XEN) xen-source/xen/common/grant_table.c:1909:d0v0 Expanding d0 \
  grant table from 1 to 2 frames

This patch changes gnttab processing from gnttab max_idx to low_idx, so the
first hypercall has the largest index, ensuring that the grant table will
grow only once. It also reduces number of log messages.

Signed-off-by: Grygorii Strashko <[email protected]>
Acked-by: Volodymyr Babchuk <[email protected]>
Reviewed-by: Dmytro Firsov <[email protected]>
@GrygiriiS GrygiriiS force-pushed the rpi5_dev_gnt_order branch from c5beeb7 to 28a74e6 Compare June 18, 2024 08:22
@firscity firscity merged commit e87418f into xen-troops:rpi5_dev Jun 25, 2024
7 checks passed
@GrygiriiS GrygiriiS deleted the rpi5_dev_gnt_order branch June 26, 2024 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants